Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimally integrate SPIR-T (opt-in via RUSTGPU_CODEGEN_ARGS=--spirt). #940

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Nov 17, 2022

Please see the SPIR-T repository for more information, but to quote the top of its README:

SPIR-🇹

⋯🢒 🇹arget 🠆 🇹ransform 🠆 🇹ranslate ⋯🢒

SPIR-🇹 is a research project aimed at exploring shader-oriented IR designs derived from SPIR-V, and producing a framework around such an IR to facilitate advanced compilation pipelines, beyond what existing SPIR-V tooling allows for.


This PR integrates it into Rust-GPU in a very limited capacity, in the middle of a sequence of passes operating on SPIR-V, as a ... -> SPIR-V -> SPIR-T -> SPIR-V -> ... conversion, with the only SPIR-T -> SPIR-T pass being the CFG structurizer (substituting our own structurizer, which we may want to delete?).

That alone is enough to show some compile-time speedups of over 30% in some workloads, while adding only marginal overhead (~0.1%), and it's still being limited by our existing passes (see EmbarkStudios/spirt#6).


To enable, you can use RUSTGPU_CODEGEN_ARGS=--spirt - to quote the (more detailed) codegen-args.md docs:

--spirt

Enables using the experimental SPIR-🇹 shader IR framework in the linker - more specifically, this:

  • adds a SPIR-V -> SPIR-🇹 -> SPIR-V roundtrip
    (future SPIR-🇹 passes would go in the middle of this, and eventually codegen might not produce SPIR-V at all)
  • replaces the existing structurizer with SPIR-🇹 structurization (which is more robust and can e.g. handle OpPhis)
  • runs some existing SPIR-V legalization/optimization passes (mem2reg) before inlining, instead of only after (as the OpPhis they would produce are no longer an issue for structurization)

For more information, also see the SPIR-🇹 repository.

--dump-spirt-passes FILE

Dump the SPIR-🇹 module across passes (i.e. all of the versions before/after each pass), as a combined report, to FILE and FILE.html.
(the .html version of the report is the recommended form for viewing, as it uses tabling for versions, syntax-highlighting-like styling, and use->def linking)


Opening this PR as a draft because there are some discussions that need to be had, i.e.:

(click to open outdated bullet points)
  • how acceptable is the lossiness we get from the SPIR-T structurizer? (see test changes)
  • should we just delete code this supersedes? (our old structurizer)
  • EDIT: the updated plan means only deleting that code when we flip SPIR-T on by default
  • should we wait for 0.4 to release, so that the SPIR-T dependency can be only 0.5 onward?
    • this would allow more time to refine it before we need to release spirt 0.1 on crates.io
    • EDIT: the updated plan has SPIR-T as opt-in for 0.4, and by default only later (0.5 or so)

@eddyb eddyb force-pushed the spirt-integration branch 2 times, most recently from 7daf64b to 05fd617 Compare November 17, 2022 22:29
@fu5ha
Copy link
Member

fu5ha commented Nov 18, 2022

  • #[spirv(unroll_loops)] is not particularly important -- we don't use it anywhere to my knowledge and gpu shader compilers should be able to do this on their own
  • the debug info is also not currently used for anything, I'm unaware of any tools that integrate well with it -- we either cross compile parts of gpu code to cpu or use shader disassemblers built into gpu debugging tools.

@oisyn

This comment was marked as outdated.

@eddyb eddyb force-pushed the spirt-integration branch from 05fd617 to 287e3c1 Compare November 22, 2022 05:20
@eddyb

This comment was marked as resolved.

@eddyb eddyb force-pushed the spirt-integration branch 2 times, most recently from e60276c to 5283b9c Compare December 3, 2022 02:54
@eddyb eddyb force-pushed the spirt-integration branch from 5283b9c to f08255e Compare December 5, 2022 22:04
eddyb added a commit to EmbarkStudios/spirt that referenced this pull request Dec 9, 2022
…meter`. (#12)

Looks like an accidental omission originally - found while trying to
align `rustc_codegen_spirv::linker::test` unit tests so SPIR-T doesn't
affect them (for
EmbarkStudios/rust-gpu#940 (comment)).
@eddyb eddyb force-pushed the spirt-integration branch from f08255e to 5091aee Compare December 9, 2022 17:24
@eddyb eddyb changed the title Minimally integrate SPIR-T (replacing structurizer and shuffling passes around). Minimally integrate SPIR-T (opt-in via RUSTGPU_CODEGEN_ARGS=--spirt). Dec 9, 2022
@eddyb eddyb force-pushed the spirt-integration branch from 5091aee to 835e444 Compare December 9, 2022 17:32
@eddyb eddyb marked this pull request as ready for review December 9, 2022 17:33
@eddyb eddyb requested a review from oisyn as a code owner December 9, 2022 17:33
@eddyb eddyb enabled auto-merge (rebase) December 9, 2022 17:33
@eddyb eddyb disabled auto-merge December 9, 2022 17:47
@eddyb eddyb marked this pull request as draft December 9, 2022 17:47
@eddyb
Copy link
Contributor Author

eddyb commented Dec 9, 2022

Ooops, had to draft again because I forgot to update the CHANGELOG.

@eddyb eddyb force-pushed the spirt-integration branch from 835e444 to b8acc42 Compare December 12, 2022 13:04
@eddyb eddyb marked this pull request as ready for review December 12, 2022 13:10
@eddyb eddyb enabled auto-merge (rebase) December 12, 2022 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants